Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search filter "No filtered data found" message added + Dropdown close/reopen issue fixed #322

Merged
merged 2 commits into from
Jan 25, 2022

Conversation

ninilo97
Copy link
Contributor

@ninilo97 ninilo97 commented Jun 13, 2021

[1]
Shows new placeholder text when no data is found during a search.
Shows :
image
Instead of
image

[2]
closeDropdown() is not required in "blur" hostlistener as it is already covered in [clickOutside] ClickOutsideDirective.
Turns out this closeDropdown was causing #198 #295 #302 and #309

Reference:
[1] Pipe filter empty check
[2] YHGxG's comment in issue thread

@ninilo97 ninilo97 changed the title Fix #122 #315 Search filter "No filtered data found" message added Jun 13, 2021
@ninilo97 ninilo97 changed the title Search filter "No filtered data found" message added Search filter "No filtered data found" message added + Dropdown close/reopen issue fixed Jun 13, 2021
@ninilo97
Copy link
Contributor Author

@NileshPatel17 Please review this pull request.

@rooby
Copy link
Contributor

rooby commented Oct 15, 2021

Generally it should be one PR per issue fixed instead of putting multiple unrelated changes in a single PR.
That way it's easier to track and revert changes if required.

Also if you're removing code, like the call to this.closeDropdown(); it should be removed, not commented out.

@rooby
Copy link
Contributor

rooby commented Jan 25, 2022

@NileshPatel17 did you mean to commit that commented out line of code instead of just deleting the line?
It gets messy with commented out code and it's not clear when reading the code why it's there and commented out.

@NileshPatel17
Copy link
Owner

@rooby agree, will remove it..

@rooby
Copy link
Contributor

rooby commented Sep 19, 2022

@NileshPatel17
I think this also fixed #295

Also, the commented out line is still there in the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants